Skip to content

feat(insert): excluded and excludedAll for onConflictDoUpdate set - #112

Merged
Makisuo merged 4 commits into
MapleTechLabs:mainfrom
artile:feat/upsert-excluded
Nov 2, 2026
Merged

Makisuo merged 4 commits into
MapleTechLabs:mainfrom
artile:feat/upsert-excluded

Conversation

@artile

@artile artile commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

A sync upsert usually copies the new row over the old one. Today each column must be written twice in onConflictDoUpdate's set callback (name: excluded.name, rent: excluded.rent, ...).

Closes #81.
Part of #52.

Changes

  • New excluded(columns): a set value that writes SET col = EXCLUDED.col for each named column.
  • New excludedAll(except?): a set value that copies every column the insert writes (the rows' keys, or the SELECT's aliases), except the named columns and the conflict target's columns. A column the rows leave out keeps its old value, because its EXCLUDED value is the default.
  • Both are typed against the table's writable columns. An unknown or computed column does not type-check. At runtime an unknown name is a QueryBuilderError InvalidArguments; an excludedAll that leaves no column fails with "sets no columns", as an empty set does.
  • Tenant scope: col = excluded.col on the tenant column now keeps the rows' tenant scope. Before, it made the insert cross-tenant, so excludedAll on a tenant table would fail under requireTenantScope. This also holds for the callback form (($, excluded) => ({ org: excluded.org })).
  • The compiler expands both helpers through one proposed-row accessor (proposedRowAccessor in src/ch/compile.ts), the same one the callback gets.
  • Docs: docs/inserts.md ("Set from the proposed row"), docs/reference.md, design/writes.md §6, design/gap-review.md (P1 item marked built). One minor changeset.

How SQLite and MySQL plug in later

  • SQLite / D1: the same excluded."col" text and the same ON CONFLICT (target) clause. The helpers work as they are once the dialect sets clauses.onConflict: true.
  • MySQL: the dialect changes only proposedRowAccessor, to write VALUES(col) (or new.col with a row alias on 8.0.19+), and renders ON DUPLICATE KEY UPDATE. It refuses an explicit target, because MySQL uses all unique keys. Then excludedAll cannot skip the target's columns from target, so it would skip the primary key instead. The helpers' public shape does not change.

Left out

  • SQLite, D1 and MySQL upsert SQL. Upstream main has no SQLite or MySQL dialect (SQLite is PR feat: add SQLite and Cloudflare D1 query dialects #49, MySQL is MySQL / MariaDB dialect #53).
  • Mixing copied columns with other values in one value (for example excludedAll plus updatedAt: now()). The callback form does this today; the docs show it. A later option can add it if you want it.
  • A dialect hook in Dialect for the proposed-row SQL. It has no second user yet, so I kept it a private function.

Validation

  • bun run build: pass.
  • bun run typecheck: pass (includes the new insert.test-d.ts cases).
  • bunx vitest run --testTimeout=60000 --hookTimeout=60000 with live ClickHouse 26.8.2.7: 60 files passed, 1039 tests passed, 0 skipped. New tests: 5 compile tests in src/ch/insert.test.ts, 1 PGlite run test in src/database/database.test.ts.
  • node scripts/check-doc-citations.mjs, node scripts/check-exports-documented.mjs, node scripts/check-doc-examples.mjs: pass.
  • bun scripts/check-package.ts: pass.
  • Upstream CI for a fork PR waits for a maintainer to approve the run.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4755e25a-eacd-43c0-a02e-93cf60496325
Commits

Reviewing files that changed from the base of the PR and between b57faf1 and d28c5fb.

Files selected for processing: 13
  • .changeset/upsert-excluded.md
  • design/gap-review.md
  • design/writes.md
  • docs/inserts.md
  • docs/reference.md
  • src/ch/compile.ts
  • src/ch/insert.test-d.ts
  • src/ch/insert.test.ts
  • src/ch/insert.ts
  • src/core.ts
  • src/database/database.test.ts
  • src/sqlite/d1.test.ts
  • src/sqlite/sqlite.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Makisuo and others added 2 commits November 2, 2026 16:31
Resolves the docs/reference.md conflict (ON CONFLICT is now Postgres and SQLite).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…proof

The SQLite dialect renders ON CONFLICT ... DO UPDATE with "excluded"."col" like
Postgres, so the helpers work there unchanged; cover them on node:sqlite and a
local D1 binding (under requireTenantScope). The tenant-scope test and the docs
now pin the conflicting row's tenant explicitly: copying the tenant column from
the proposed row does not prove which tenant the existing row belongs to.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Makisuo

Makisuo commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Maintainer pass for merge-readiness (head d28c5fb):

Changes

  • Merged origin/main (SQLite/D1 dialects, Effect 4.0.6, afterCommit, fuzz). Only conflict: the insertInto row in docs/reference.md; it now says ON CONFLICT works on Postgres and SQLite.
  • SQLite/D1: the SQLite dialect writes ON CONFLICT … DO UPDATE with "excluded"."col", the same as Postgres, so excluded/excludedAll work there unchanged. Added run tests on node:sqlite (excluded, plus excludedAll keeping a column the rows leave out) and on a local Miniflare D1 binding under requireTenantScope. The changeset and docs now name Postgres, SQLite and D1.
  • Tenant scope: org = excluded.org stops making the insert cross-tenant. That is right for the inserted rows, but it proves nothing about the tenant of the existing conflicting row. That gap is Tenant guard: ON CONFLICT(id) DO UPDATE can modify another tenant while classified single-tenant #127, which fix: guard upsert tenant scope and SQLite rebuild dependencies #130 fixes. The new tenant test and the D1 test now pin the existing row's tenant with where: (existing, excluded) => existing.org.eq(excluded.org), so they hold before and after fix: guard upsert tenant scope and SQLite rebuild dependencies #130. The docs tell users to put the tenant column in target.

Interaction with #130

Verification

  • bun run build and bun run typecheck pass.
  • bun run test against live ClickHouse 26.8: 73 files and 1437 tests pass, and the doc citation, export catalog and doc example checks pass.
  • FUZZ_SCALE=0.3 bunx vitest run tests/fuzz: 9 files and 269 tests pass.

@Makisuo
Makisuo merged commit 0e39f67 into MapleTechLabs:main Nov 2, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Portable upsert on SQLite and MySQL, plus an excluded helper

2 participants